Skip to content

fix(extract): keep decorators separated by a comment#1097

Open
JhohanBustamante wants to merge 1 commit into
DeusData:mainfrom
JhohanBustamante:fix/decorator-comment-drop
Open

fix(extract): keep decorators separated by a comment#1097
JhohanBustamante wants to merge 1 commit into
DeusData:mainfrom
JhohanBustamante:fix/decorator-comment-drop

Conversation

@JhohanBustamante

Copy link
Copy Markdown
Contributor

Fixes #1095.

A comment between decorators drops every decorator above it

extract_decorators() walks prev-siblings and breaks on the first NAMED node.
Comments are named nodes in tree-sitter, so a comment interleaved in a
decorator run ends the walk:

@Post('login')                     // <-- dropped
@HttpCode(HttpStatus.OK)           // <-- dropped
// throttled per IP and per account
@Throttle({ default: { ttl: 900_000, limit: 5 } })   // <-- kept
async login(dto: LoginDto) { ... }

The route then has no @Post, so it vanishes from any decorator-based query:
documenting a decorator removes the endpoint from the graph. On the backend I was
indexing, exactly 1 of 95 HTTP endpoints was missing — the one whose throttle
policy carried an explanatory comment.

Comments are now transparent to the walk, the same way the anonymous tokens
(e.g. TS export) already were. Reuses the existing is_comment_node() helper.

Test

  • tests/test_extraction.c: extract_ts_decorators_survive_interleaved_comment

test-runner extraction → 214 passed (ASan + UBSan build).


Split out of #1075 per review, as one focused PR linked to its issue. The Cypher
composite-property fix is now #1096 / its own PR.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges labels Jul 15, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 15, 2026
@DeusData DeusData added the priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. label Jul 15, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for splitting this into a focused PR. The current head fb7a808 is atomic, the regression is red on the previous named-comment break behavior, and both counting and collection walks apply the same boundary rule. Non-comment named siblings still terminate the run, so this does not broaden decorator association across declarations.

The full CI and DCO sets are green. The security review found no dependency, workflow, network, credential, prompt-injection, or parser-execution risk. I have triaged it as a high-priority 0.9.1-rc parsing fix. No contributor change is requested at this stage; the next step is final maintainer diff review.

@DeusData

Copy link
Copy Markdown
Owner

Reviewed — this is a clean, well-scoped fix and the extract_ts_decorators_survive_interleaved_comment test is a proper reproduce-first guard (it genuinely fails on main without the symmetric !is_comment_node guard). The extract_defs.c change is exactly right and merges cleanly.

One thing before it can land: the branch base is ~29 commits behind main, and there's a mechanical conflict in tests/test_extraction.cmain has since appended more RUN_TEST(...) lines to the tail of SUITE(extraction) at the same spot your RUN_TEST(extract_ts_decorators_survive_interleaved_comment) lands. Could you rebase (or merge main into your branch) and re-anchor that one registration line? The code change itself is conflict-free. Once it's green I'll merge (with a merge commit, to keep your DCO sign-off intact). Thanks @JhohanBustamante!

Comments are NAMED tree-sitter nodes, so the prev-sibling walk in
extract_decorators() stopped at one — silently dropping every decorator ABOVE
an interleaved comment:

    @post('login')                 <-- dropped
    @httpcode(HttpStatus.OK)       <-- dropped
    // throttled per IP and account
    @Throttle({ ... })             <-- kept
    async login(...)

The route then vanished from decorator/route queries, so documenting a
decorator made the endpoint disappear from the graph. Real-world impact: on a
NestJS backend, 1 of 95 HTTP endpoints was missing — the one whose throttle
policy carried an explanatory comment.

Comments are now transparent to the walk, the same way anonymous tokens
(e.g. TS `export`) already were. Reuses the existing is_comment_node() helper.

Signed-off-by: KolisCode <jhohantma@gmail.com>
@JhohanBustamante JhohanBustamante force-pushed the fix/decorator-comment-drop branch from fb7a808 to 012be57 Compare July 16, 2026 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TS/JS extraction: a comment interleaved in a decorator run drops every decorator above it

2 participants